Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go-docappender: use a fixed-size buffer #129

Closed
wants to merge 1 commit into from
Closed

Conversation

vikmik
Copy link
Contributor

@vikmik vikmik commented Feb 21, 2024

This is an alternative implementation of PR #124 that doesn't require expanding the buffer, and requires fewer variables to track positions (hopefully improving maintainability)

@vikmik vikmik requested a review from a team as a code owner February 21, 2024 06:12
@elastic-apm-tech elastic-apm-tech added the safe-to-test Automated label for running bench-diff on forked PRs label Feb 21, 2024
@vikmik vikmik force-pushed the vic-124-alternative branch 3 times, most recently from d38b04b to fb07dd0 Compare February 23, 2024 21:03
Comment on lines +237 to +238
n := copy(b.copyBuf, b.buf.Bytes())
b.copyBuf = b.copyBuf[:n]
Copy link
Contributor Author

@vikmik vikmik Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to this PR there was no error handling of the gzip reader.
I fixed that below, but that required resizing down copyBuf here when necessary - otherwise there might be unrelated data from previous flushes, causing failed gzip reads.

@vikmik vikmik force-pushed the vic-124-alternative branch from fb07dd0 to 401a4b3 Compare February 28, 2024 02:20
@vikmik
Copy link
Contributor Author

vikmik commented Mar 5, 2024

Closing in favor of #124

@vikmik vikmik closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe-to-test Automated label for running bench-diff on forked PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants